I wonder if we want to make {{section}} optional and if it does not exists then append section at the end. (Which seem to be the current logic)

Of if we just want to treat is as required if want to use it.
i.e. should we support url like http://example.com?lang=en that would not include the section.
Otherwise, the current implementation will always append the section so http://example.com?lang=en would turn into http://example.com?lang=en/section which would cause issues. And to get around this we would need to add http://example.com?lang=en&dummy={{section}} to add a dummy section that is not used.

If we do want {{section}} to be mandatory to be used then I suspect that the section will need to be added to existing urls

UPDATE Settings SET value = CONCAT(value, '{{section}}') WHERE name = 'system/documentation/url';

Also you will need to update the following.

INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/documentation/url', 'https://docs.geonetwork-opensource.org/{{version}}/{{lang}}', 0, 570, 'n');
To

INSERT INTO Settings (name, value, datatype, position, internal) VALUES ('system/documentation/url', 'https://docs.geonetwork-opensource.org/{{version}}/{{lang}}/{{section}}', 0, 570, 'n');

What do other think?

@ianwallen I think it is fine if the placeholder is optional

The change works, but with the current code this kind of URL doesn't work:

https://something/subsection?lang=eng

The lang placeholder replacement checks for /{{lang}} instead of {{lang}}. This is not changed in the PR, but not sure why the original code has the /.

The official manual adds the language as part of the URL path, not as a parameter:

https://docs.geonetwork-opensource.org/4.4/fr/overview/

I assume you have some customised documentation that uses language as a parameter?

@josegar74

The original has / is because it doesn't like to parse English into the default document.

For example:

https://docs.geonetwork-opensource.org/4.2/user-guide/describing-information/creating-metadata/

vs French version

https://docs.geonetwork-opensource.org/4.2/fr/user-guide/describing-information/creating-metadata/.

Now if we ignore that "/" logic, we will end up with default English url as:
https://docs.geonetwork-opensource.org/4.2//user-guide/describing-information/creating-metadata/ which seems to be fine. I could make such logic by ignoring such slash

All committers have signed the CLA.

I would keep the replacement to empty string and replace // in the URL path.

Please check this change to apply it in your code:

          /**
           * Processes an URL removing // characters in the URL path.
           *
           * @param url
           * @returns {string}
           */
          var processUrl = function(url) {
            var urlToProcess = new URL(url);
            urlToProcess.pathname = urlToProcess.pathname.replace(/\/\//g, "/");
            return urlToProcess.toString();
          }

          scope.showHelp = function () {
          ...
           var helpPageUrl;
            if (baseUrl.includes("{{section}}")) {
              helpPageUrl = baseUrl.replace("{{section}}", page);
            } else {
              helpPageUrl = baseUrl + "/" + page;
            }

            helpPageUrl = processUrl(helpPageUrl);

            testAndOpen(helpPageUrl).then(
              function () {},
              function () {
                var baseUrl = scope.helpBaseUrl
                  .replace("{{lang}}", "")
                  .replace("{{version}}", scope.applicationVersion);
                var helpPageUrl;
                if (baseUrl.includes("{{section}}")) {
                  helpPageUrl = baseUrl.replace("{{section}}", page);
                } else {
                  helpPageUrl = baseUrl + "/" + page;
                }

                helpPageUrl = processUrl(helpPageUrl);

                testAndOpen(helpPageUrl);
              }
              ...

@josegar74

This approach works fine. I did apply this into the code, please check

@wangf1122 you need to call the method also before https://github.com/geonetwork/core-geonetwork/pull/8494/files#diff-4de39770e16feb2f9abebba5ec7abc50c694b7d2ee7bfece5cd558b27ca986a6R194, in the error handler.

@josegar74

Do you mean here? Please check this commit aa1e4ce

@wangf1122 I'm not sure what happened, but previously I missed that line. Looks all good.

The backport to 4.2.x failed:

The process '/usr/bin/git' failed with exit code 1

error: could not apply 9901adac8f... support section variable in manual url
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config advice.mergeConflict false"

Auto-merging web-ui/src/main/resources/catalog/locales/en-admin.json
CONFLICT (content): Merge conflict in web-ui/src/main/resources/catalog/locales/en-admin.json

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.2.x 4.2.x
# Navigate to the new working tree
cd .worktrees/backport-4.2.x
# Create a new branch
git switch --create backport-8494-to-4.2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 9901adac8f7b701ce5a8567381de94190032d9cd,0b000ce24b95fa22429836ed06a6a4bd8a68f16f,2e55b577807206f3fa9c79346c31dea2c5dd61f7,455dba0fd01cb09ad7ee284ca55b5a50353f7bc6,d50d853e56b034d84cef52d4c11339ea3dfc72d0,907583992ef95cb798aa78d3343a83ebf6b16d0c,a3b22a60c1a9b735877fcbb9f01494ebae777cc4,aa1e4cecceca522d5bcaf7fd7e1059a69d58bb29
# Push it to GitHub
git push --set-upstream origin backport-8494-to-4.2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.2.x

Then, create a pull request where the base branch is 4.2.x and the compare/head branch is backport-8494-to-4.2.x.